Fix NPE during kubernetes cluster creation when network has rules with ports saved as null on DB #9223

GaOrtiga · 2024-06-11T14:13:19Z

Description

During the creation of firewall rules, if one of the limits for the ports is not informed, it is saved in the database as Null, indicating that there is no starting/ending limit.

while creating a Kubernetes cluster, if the selected network has a rule that contains ports saved as null, an error is thrown, stopping the execution of the process.

This behaviour has been fixed, making it so that any port saved as null is regarded as being being on its respective limit (1 for start ports and 65535 for end ports) during the creation of a Kubernetes cluster.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI

Feature/Enhancement Scale or Bug Severity

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

I created a rule in an existing network, with null starting port and ending on port 10, making it so it should not conflict with any of the ports required by the Kubernetes cluster. Before applying the changes, an error would be thrown.

I repeated the process after applying the changes and the cluster was successfully created.

...ernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java

DaanHoogland

clgtm

.../src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java

codecov · 2024-06-12T14:09:03Z

Codecov Report

Attention: Patch coverage is 13.04348% with 20 lines in your changes missing coverage. Please review.

Project coverage is 15.81%. Comparing base (b19c069) to head (b3be45e).

Files with missing lines	Patch %	Lines
...va/com/cloud/network/dao/FirewallRulesDaoImpl.java	0.00%	11 Missing ⚠️
...KubernetesClusterResourceModifierActionWorker.java	0.00%	8 Missing ⚠️
...om/cloud/network/firewall/FirewallManagerImpl.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9223      +/-   ##
============================================
- Coverage     15.81%   15.81%   -0.01%     
+ Complexity    12554    12553       -1     
============================================
  Files          5629     5629              
  Lines        492028   492041      +13     
  Branches      62750    60065    -2685     
============================================
- Hits          77813    77812       -1     
- Misses       405892   405906      +14     
  Partials       8323     8323

Flag	Coverage Δ
uitests	`4.48% <ø> (ø)`
unittests	`16.60% <13.04%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

...ernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java

weizhouapache

code lgtm

not tested

borisstoyanov · 2024-06-13T10:33:18Z

@blueorangutan package

blueorangutan · 2024-06-13T10:34:04Z

@borisstoyanov a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-06-13T11:20:35Z

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9927

borisstoyanov

I think there's a failing test @GaOrtiga

13:56:56 [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.128 s - in com.cloud.kubernetes.version.KubernetesVersionManagerImplTest
13:56:56 [INFO] Running com.cloud.kubernetes.version.KubernetesVersionServiceTest
13:56:56 [INFO] Tests run: 9, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.234 s - in com.cloud.kubernetes.version.KubernetesVersionServiceTest
13:56:57 [INFO] 
13:56:57 [INFO] Results:
13:56:57 [INFO] 
13:56:57 [ERROR] Failures: 
13:56:57 [ERROR]   KubernetesClusterManagerImplTest.validateIsolatedNetworkIpRulesApiConflictingRules Expected exception: com.cloud.exception.InvalidParameterValueException
13:56:57 [ERROR]   KubernetesClusterManagerImplTest.validateIsolatedNetworkIpRulesSshConflictingRules Expected exception: com.cloud.exception.InvalidParameterValueException
13:56:57 [INFO] 
13:56:57 [ERROR] Tests run: 39, Failures: 2, Errors: 0, Skipped: 0
13:56:57 [INFO] 
13:56:57 [INFO] ------------------------------------------------------------------------
13:56:57 [INFO] Reactor Summary for Apache CloudStack 4.20.0.0-SNAPSHOT:

borisstoyanov · 2024-06-13T12:19:51Z

thanks @GaOrtiga
@blueorangutan package

blueorangutan · 2024-06-13T12:22:04Z

@borisstoyanov a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-06-13T13:41:23Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9929

FelipeM525

I tested this PR in a local environment by trying to create a k8s cluster on a network with a firewall rule containing an empty start port.

Before

After

DaanHoogland · 2024-07-16T18:48:56Z

@blueorangutan package

blueorangutan · 2024-07-16T18:50:03Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-07-16T20:02:18Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10382

DaanHoogland · 2024-07-17T06:27:49Z

@blueorangutan test

blueorangutan · 2024-07-17T06:30:03Z

@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2024-07-17T21:11:18Z

[SF] Trillian test result (tid-10884)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 51258 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9223-t10884-kvm-centos7.zip
Smoke tests completed. 129 look OK, 8 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_role_account_acls_multiple_mgmt_servers	`Error`	2.15	test_dynamicroles.py
test_query_async_job_result	`Error`	102.01	test_async_job.py
test_revoke_certificate	`Error`	0.02	test_certauthority_root.py
test_configure_ha_provider_invalid	`Error`	0.01	test_hostha_simulator.py
test_configure_ha_provider_valid	`Error`	0.04	test_hostha_simulator.py
test_ha_configure_enabledisable_across_clusterzones	`Error`	0.01	test_hostha_simulator.py
test_ha_disable_feature_invalid	`Error`	0.01	test_hostha_simulator.py
test_ha_enable_feature_invalid	`Error`	0.01	test_hostha_simulator.py
test_ha_list_providers	`Error`	0.01	test_hostha_simulator.py
test_ha_multiple_mgmt_server_ownership	`Error`	0.01	test_hostha_simulator.py
test_ha_verify_fsm_available	`Error`	0.01	test_hostha_simulator.py
test_ha_verify_fsm_degraded	`Error`	0.01	test_hostha_simulator.py
test_ha_verify_fsm_fenced	`Error`	0.01	test_hostha_simulator.py
test_ha_verify_fsm_recovering	`Error`	0.01	test_hostha_simulator.py
test_hostha_configure_default_driver	`Error`	0.01	test_hostha_simulator.py
test_hostha_configure_invalid_provider	`Error`	0.01	test_hostha_simulator.py
test_hostha_disable_feature_valid	`Error`	0.01	test_hostha_simulator.py
test_hostha_enable_feature_valid	`Error`	0.01	test_hostha_simulator.py
test_hostha_enable_feature_without_setting_provider	`Error`	0.01	test_hostha_simulator.py
test_list_ha_for_host	`Error`	0.01	test_hostha_simulator.py
test_list_ha_for_host_invalid	`Error`	0.01	test_hostha_simulator.py
test_list_ha_for_host_valid	`Error`	0.01	test_hostha_simulator.py
test_01_host_ping_on_alert	`Error`	0.07	test_host_ping.py
test_01_host_ping_on_alert	`Error`	0.07	test_host_ping.py
test_01_browser_migrate_template	`Error`	15.30	test_image_store_object_migration.py
test_06_purge_expunged_vm_background_task	`Failure`	340.83	test_purge_expunged_vms.py
test_hostha_enable_ha_when_host_disabled	`Error`	5.79	test_hostha_kvm.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	305.91	test_hostha_kvm.py

blueorangutan · 2024-07-19T04:29:45Z

[SF] Trillian test result (tid-10912)
Environment: kvm-alma8 (x2), Advanced Networking with Mgmt server a8
Total time taken: 52725 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9223-t10912-kvm-alma8.zip
Smoke tests completed. 135 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_06_purge_expunged_vm_background_task	`Failure`	339.14	test_purge_expunged_vms.py
test_03_secured_to_nonsecured_vm_migration	`Error`	396.95	test_vm_life_cycle.py

DaanHoogland · 2024-07-19T08:15:21Z

@borisstoyanov , @FelipeM525 tested (#9223 (review)), do you still want to test this as well?

JoaoJandre · 2024-08-13T19:34:10Z

@blueorangutan package

blueorangutan · 2024-08-13T19:36:05Z

@JoaoJandre a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-08-13T20:45:20Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10641

blueorangutan · 2024-08-14T14:06:22Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10658

rohityadavcloud · 2024-09-04T06:43:42Z

@GaOrtiga can you review and address build failures? Thanks.

GaOrtiga · 2024-09-11T17:42:27Z

@blueorangutan package

blueorangutan · 2024-09-11T17:44:03Z

@GaOrtiga a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-09-11T18:49:42Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11098

weizhouapache · 2024-09-11T19:11:43Z

@GaOrtiga
Can you update the title with short summay of the issue ?

GaOrtiga · 2024-09-12T16:59:11Z

@GaOrtiga can you review and address build failures? Thanks.

I have rebased it, everything seems fine now.

@GaOrtiga Can you update the title with short summay of the issue ?

Title updated.

weizhouapache · 2024-09-12T17:57:45Z

@blueorangutan package

blueorangutan · 2024-09-12T18:00:03Z

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-09-12T19:00:57Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11103

DaanHoogland · 2024-09-16T07:42:52Z

@blueorangutan test

blueorangutan · 2024-09-16T07:44:03Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2024-09-16T23:23:42Z

[SF] Trillian test result (tid-11481)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 53922 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9223-t11481-kvm-ol8.zip
Smoke tests completed. 141 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland · 2024-09-17T06:46:40Z

@borisstoyanov is this lgty now?

boring-cyborg bot added the component:kubernetes label Jun 11, 2024

weizhouapache reviewed Jun 11, 2024

View reviewed changes

...ernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java Show resolved Hide resolved

DaanHoogland approved these changes Jun 11, 2024

View reviewed changes

.../src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java Show resolved Hide resolved

boring-cyborg bot added the component:networking label Jun 12, 2024

GaOrtiga force-pushed the fix_NPE_Kubernetes_cluster branch 2 times, most recently from 1be11ff to a8ce0f2 Compare June 12, 2024 18:35

weizhouapache reviewed Jun 12, 2024

View reviewed changes

...ernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java Outdated Show resolved Hide resolved

weizhouapache approved these changes Jun 12, 2024

View reviewed changes

weizhouapache added the status:needs-testing label Jun 12, 2024

borisstoyanov self-assigned this Jun 13, 2024

This comment was marked as outdated.

Sign in to view

borisstoyanov reviewed Jun 13, 2024

View reviewed changes

borisstoyanov self-requested a review June 13, 2024 11:43

FelipeM525 approved these changes Jul 16, 2024

View reviewed changes

Gabriel added 5 commits September 11, 2024 14:19

fix NPE during kubernetes cluster creation

328cc95

fix delete

b8470ec

skip other protocols

738c53b

use NetUtils.TCP_PROTO instead

921190d

fix tests

b3be45e

GaOrtiga force-pushed the fix_NPE_Kubernetes_cluster branch from 97e3988 to b3be45e Compare September 11, 2024 17:41

GaOrtiga changed the title ~~fix error during kubernetes cluster creation~~ Fix NPE during kubernetes cluster creation when ports are saved as null on DB Sep 12, 2024

GaOrtiga changed the title ~~Fix NPE during kubernetes cluster creation when ports are saved as null on DB~~ Fix NPE during kubernetes cluster creation when network has rules with ports saved as null on DB Sep 12, 2024

Fix NPE during kubernetes cluster creation when network has rules with ports saved as null on DB #9223

Are you sure you want to change the base?

Fix NPE during kubernetes cluster creation when network has rules with ports saved as null on DB #9223

Conversation

GaOrtiga commented Jun 11, 2024

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

DaanHoogland left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 12, 2024 • edited Loading

Codecov Report

weizhouapache left a comment

Choose a reason for hiding this comment

borisstoyanov commented Jun 13, 2024

blueorangutan commented Jun 13, 2024

blueorangutan commented Jun 13, 2024

This comment was marked as outdated.

borisstoyanov left a comment

Choose a reason for hiding this comment

borisstoyanov commented Jun 13, 2024

blueorangutan commented Jun 13, 2024

blueorangutan commented Jun 13, 2024

FelipeM525 left a comment

Choose a reason for hiding this comment

DaanHoogland commented Jul 16, 2024

blueorangutan commented Jul 16, 2024

blueorangutan commented Jul 16, 2024

DaanHoogland commented Jul 17, 2024

blueorangutan commented Jul 17, 2024

blueorangutan commented Jul 17, 2024

blueorangutan commented Jul 19, 2024

DaanHoogland commented Jul 19, 2024

JoaoJandre commented Aug 13, 2024

blueorangutan commented Aug 13, 2024

blueorangutan commented Aug 13, 2024

blueorangutan commented Aug 14, 2024

rohityadavcloud commented Sep 4, 2024

GaOrtiga commented Sep 11, 2024

blueorangutan commented Sep 11, 2024

blueorangutan commented Sep 11, 2024

weizhouapache commented Sep 11, 2024

GaOrtiga commented Sep 12, 2024

weizhouapache commented Sep 12, 2024

blueorangutan commented Sep 12, 2024

blueorangutan commented Sep 12, 2024

DaanHoogland commented Sep 16, 2024

blueorangutan commented Sep 16, 2024

blueorangutan commented Sep 16, 2024

DaanHoogland commented Sep 17, 2024

codecov bot commented Jun 12, 2024 •

edited

Loading